Skip to content

Support Prometheus UNIT metadata#3651

Merged
CodeBlanch merged 3 commits intoopen-telemetry:mainfrom
reyang:reyang/prometheus-unit
Sep 13, 2022
Merged

Support Prometheus UNIT metadata#3651
CodeBlanch merged 3 commits intoopen-telemetry:mainfrom
reyang:reyang/prometheus-unit

Conversation

@reyang
Copy link
Copy Markdown
Member

@reyang reyang commented Sep 13, 2022 •
edited
Loading

Changes

  • Added support for UNIT metadata.
  • Renamed some of the internal methods to better align with OpenMetrics terms (e.g. "metadata").
  • Adjusted the ordering of the metadata to follow the OpenMetrics best practices.

Reference: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#overall-structure

image

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@reyang reyang requested a review from a team September 13, 2022 16:56
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 13, 2022 •
edited
Loading

Codecov Report

Merging #3651 (3d1fb73) into main (de98eb7) will increase coverage by 0.02%.
The diff coverage is 94.73%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3651      +/-   ##
==========================================
+ Coverage   87.58%   87.60%   +0.02%     
==========================================
  Files         283      283              
  Lines       10218    10234      +16     
==========================================
+ Hits         8949     8966      +17     
+ Misses       1269     1268       -1
Impacted Files Coverage Δ
...heus.HttpListener/Internal/PrometheusSerializer.cs 80.34% <93.75%> (+2.12%) ⬆️
...s.HttpListener/Internal/PrometheusSerializerExt.cs 100.00% <100.00%> (ø)
...tpListener/Internal/PrometheusCollectionManager.cs 78.04% <0.00%> (-2.44%) ⬇️
...ZPages/Implementation/ZPagesExporterEventSource.cs 62.50% <0.00%> (+6.25%) ⬆️
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs 82.35% <0.00%> (+8.82%) ⬆️

@reyang reyang added pkg:OpenTelemetry.Exporter.Prometheus.AspNetCore Issues related to OpenTelemetry.Exporter.Prometheus.AspNetCore NuGet package enhancement New feature or request labels Sep 13, 2022
cijothomas
cijothomas reviewed Sep 13, 2022
View reviewed changes
test/OpenTelemetry.Exporter.Prometheus.Tests/PrometheusSerializerTests.cs
.AddInMemoryExporter(metrics)
.Build();

meter.CreateObservableGauge("test_gauge", () => 123, unit: "seconds", description: "Hello, world!");
Copy link
Copy Markdown
Member

@cijothomas cijothomas Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice description :)

Copy link
Copy Markdown
Member

@CodeBlanch CodeBlanch Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some clever Greek mythology thing. Defer to @alanwest to come up with it 🤣

Copy link
Copy Markdown
Member

@alanwest alanwest Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, would "Hello, Prometheus!" not have been clever enough? And also what the heck do I know about Greek mythology?

Copy link
Copy Markdown
Member Author

@reyang reyang Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we consider the Prometheus Exporters to be renamed to OpenMetrics Exporters? 😆

CodeBlanch
CodeBlanch approved these changes Sep 13, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@CodeBlanch CodeBlanch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@CodeBlanch CodeBlanch merged commit 6ea078e into open-telemetry:main Sep 13, 2022
@reyang reyang deleted the reyang/prometheus-unit branch September 13, 2022 18:26
@reyang reyang mentioned this pull request Sep 13, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alanwest alanwest alanwest left review comments

@cijothomas cijothomas cijothomas left review comments

@CodeBlanch CodeBlanch CodeBlanch approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request pkg:OpenTelemetry.Exporter.Prometheus.AspNetCore Issues related to OpenTelemetry.Exporter.Prometheus.AspNetCore NuGet package

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@reyang @alanwest @cijothomas @CodeBlanch